Conversation
witoszekdev
commented
Apr 2, 2026
witoszekdev
commented
- add back JSONParse helper
- add changeset
🦋 Changeset detectedLatest commit: b6a3867 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 9 Skipped Deployments
|
There was a problem hiding this comment.
Pull request overview
This PR re-introduces the handlebars-helpers JSON parsing helper into the @saleor/handlebars allow-list and updates the helper-registration tests accordingly, alongside adding a changeset for release notes/versioning.
Changes:
- Re-enabled the
objecthelpers group partially by allowingJSONparsewhile keepingextend/mergedisallowed. - Added a focused test asserting dangerous
objecthelpers (extend,merge) are not registered. - Added a changeset entry describing the helper being re-added.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/handlebars/src/register-allowed-helpers.test.ts | Updates tests to reflect the object group no longer being fully removed and adds assertions for extend/merge not being registered. |
| packages/handlebars/src/allowed-helpers.ts | Re-adds the object group with JSONparse to the allow-list while documenting why other object helpers remain removed. |
| .changeset/sixty-sloths-win.md | Adds a changeset entry describing the helper re-addition (currently targets the wrong package and uses inconsistent helper casing). |
| // --- object (extend, merge removed – prototype pollution risk) --- | ||
| object: ["JSONparse"], |
There was a problem hiding this comment.
ALLOWED_HELPERS now whitelists the object group with JSONparse, which changes behavior for downstream consumers. In apps/smtp/src/modules/smtp/services/handlebars-template-compiler.test.ts (lines 103-114) there is an explicit security regression test asserting JSONparse is not available; with this allow-list change it will become registered and that test (and the intended security posture) will break. Either keep JSONparse disallowed, or update SMTP’s threat model/tests and consider adding additional safeguards (e.g., rejecting __proto__/constructor/prototype keys) if templates can be user-controlled.
| // --- object (extend, merge removed – prototype pollution risk) --- | |
| object: ["JSONparse"], | |
| // --- object: removed entirely (JSONparse disabled for security) --- |
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| "saleor-app-smtp": patch | |||
There was a problem hiding this comment.
This changeset bumps saleor-app-smtp, but the code change is in @saleor/handlebars (see packages/handlebars/package.json). As written, the release will publish a version bump/changelog for the wrong package and won’t version the helper change correctly.
| "saleor-app-smtp": patch | |
| "@saleor/handlebars": patch |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
apps/smtp/src/modules/smtp/services/handlebars-template-compiler.test.ts
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2319 +/- ##
=======================================
Coverage 37.30% 37.30%
=======================================
Files 1018 1018
Lines 65972 65972
Branches 3400 3400
=======================================
Hits 24608 24608
Misses 40988 40988
Partials 376 376
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
